Add filtering, paging, and richer output to all list commands - #9

Thanks for the PR — the direction is right, and there's a genuine bug fix in here. That said, I don't think it's mergeable as-is: one hard CI blocker, one functional bug, and several undocumented breaking changes.

What's good (verified)

• view list really was broken on main. Per the TM1py source, ViewService.get_all_names() returns a Tuple[List[str], List[str]] (private, public); the old code iterated over that tuple. Reproduced on main, the output was:

  ['Priv1']
  ['View1', 'View2']
  

  This was invisible only because MockedViewService returned a flat list — so the mock fix in conftest.py is correct and uncovers a real defect.
• All the changed TM1py calls match the actual signatures (skip_control_cubes / skip_control_dims / skip_control_processes, subsets.get_all_names(dimension_name, hierarchy_name, private)).
• --skip-control-cubes → --skip-control-dims on dimension list: a real copy-paste fix.
• The threads --beautify IndexError guard: reproducible crash on an empty thread list, cleanly fixed.
• Switching from rich.print to typer.echo is the right call here — Rich would interpret TM1 names containing square brackets as markup and mangle the YAML/JSON. This is a deliberate departure from the convention in CLAUDE.md and should be recorded there as an exception.

Blockers

1. The pylint CI job fails

main scores 10.00/10 (exit 0); this branch scores 8.90/10, exit 28 → .github/workflows/pylint.yml would go red. 29 findings:

⚠️ The workflow is triggered on: [push], which does not fire for fork PRs in the upstream repo (this PR currently has 0 check runs). So the failure would only become visible after the merge to main.

Suggested fix: rename the parameters internally to name_filter / output_format (the CLI surface stays identical via typer.Option("--filter", "-f", ...) and typer.Option("--type", "-t", ...)), wrap the long lines, drop the now-unused print import, and add a # pylint: disable=invalid-name in list_utils.py.

2. Bug: --hierarchy without --dimension

In tm1cli/commands/subset.py, hier = hierarchy or dim is evaluated while iterating over all dimensions. So tm1cli subset list --hierarchy Leaves queries the hierarchy Leaves on every dimension — against a real server that raises exceptions/404s for nearly all of them. The mock hides this because it ignores hierarchy_name.

--hierarchy should require --dimension and otherwise bail out via print_error_and_exit.

3. Breaking changes with no documentation

git diff --name-only main..HEAD shows that README, CHANGELOG and pyproject.toml are untouched. Affected:

• subset list DIMENSION_NAME (positional) → subset list --dimension DIMENSION_NAME — README line 65 still shows the old syntax
• view list CUBE_NAME (positional) → view list --cube CUBE_NAME — README line 62 likewise
• Output format of all five list commands: Name → - Name
• dimension list --skip-control-cubes → --skip-control-dims

For a package published on PyPI (currently 0.2.0) this needs a ### Breaking changes section in the CHANGELOG and a version bump — see "Releasing" in CLAUDE.md.

Other findings

4. --limit accepts negative values

--limit -1 silently returns every entry except the last (items[:-1]) instead of erroring:

$ tm1cli cube list --limit -1
- Cube1          # Cube2 disappears with no message

--offset correctly sets min=0; --limit is missing min=1.

5. Silent regex fallback in apply_filter

On re.error the function falls back to substring matching without a word. cube list --filter "cube[" returns [] with no hint that the pattern was invalid. On top of that: TM1 object names frequently contain (, ), . — so --filter "Sales(EMEA)" is interpreted as a regex with a group and does not match the literal name.

Suggestion: define --filter as substring/glob and offer a separate --regex; at the very least, abort via print_error_and_exit on an invalid regex rather than silently switching semantics.

6. Inconsistent handling of control objects

cube list and dimension list show control objects by default (-s is opt-in), but view list and subset list hard-wire skip_control_cubes=True / skip_control_dims=True with no flag to include them. Pick one: opt-in everywhere, or opt-out everywhere.

7. The global --output-raw is ignored by the list commands

There are now two independent output-control mechanisms (--output-raw on the callback, --output per command), and no way back to plain-text output. tm1cli cube list | while read name; do ...; done breaks because of the - prefix. Suggestion: either add --output plain or honour ctx.obj["raw"] in the list commands.

8. N+1 requests on the new default paths

view list without --cube issues 1 + 2 × (number of cubes) REST calls — TM1py fetches private and public views in separate requests, even for --type public. subset list without --dimension does the same per dimension. On large models this is noticeably slow, with no progress indication. Worth mentioning in the help text at minimum.

9. Test gaps

The 28 mocked tests pass for me — but the new flags are covered only for process. Not covered:

• --filter / --limit / --offset / --output on cube, dimension, subset, view
• --type private and --type both
• all the -s flags (the mocks ignore skip_control_* anyway, so a test wouldn't actually assert anything)
• the threads empty-list fix

Also, MockedSubsetService returns the same names for private=True as for private=False, so --type both produces duplicates in the tests without that standing out.

10. Two claims in the PR description don't hold

• The claimed fix "duplicate Annotated import from typing_extensions removed" doesn't apply — main already imports exclusively from typing.
• The subset records also carry a hierarchy field, which the description doesn't mention.

Nits

• output: Annotated[OutputFormat, ...] = "yaml" — better to default to the enum member OutputFormat.yaml than to the raw string.
• --cube has the short flag -c, --dimension has none. Deliberate, because of the clash with -d (= database)? If so, maybe -D.
• subset.py and view.py share ~10 identical lines of filter/render logic (this is what triggers R0801) → consider lifting it into list_utils.py, e.g. as filter_records(records, key, pattern).
• tests/test_tm1cli.py:67 is 133 characters long (not linted, but out of step with the surrounding style).
• The PR head is the fork's main branch — a feature branch would be more practical for follow-ups.

Generated by Claude Code